Skip to content

* 2134 feature request implement a way of getting a way to show keyboard shortcuts#2143

Closed
PWagner1 wants to merge 14 commits intoalphafrom
2134-feature-request-implement-a-way-of-getting-a-way-to-show-keyboard-shortcuts
Closed

* 2134 feature request implement a way of getting a way to show keyboard shortcuts#2143
PWagner1 wants to merge 14 commits intoalphafrom
2134-feature-request-implement-a-way-of-getting-a-way-to-show-keyboard-shortcuts

Conversation

@PWagner1
Copy link
Contributor

@PWagner1 PWagner1 commented Mar 9, 2025

@giduac
Copy link
Contributor

giduac commented Mar 9, 2025

@PWagner1

Can you please provide an overview of how this works on the discord channel where we discuss the theming changes.
Thus we keep an overview what is being implemented.

@PWagner1
Copy link
Contributor Author

PWagner1 commented Mar 9, 2025

@PWagner1

Can you please provide an overview of how this works on the discord channel where we discuss the theming changes. Thus we keep an overview what is being implemented.

@giduac

Just pinged you on Discord with a sample

// Extract Fonts
XmlNodeList fontNodes = root.SelectNodes("Resources/Fonts/Font");
foreach (XmlNode fontNode in fontNodes)
XmlNodeList? fontNodes = root.SelectNodes("Resources/Fonts/Font");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a Base64Coded string. Font has a toString and from string conversion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smurf-IV & @giduac

Should only the images be encoded as base64?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smurf-IV & @PWagner1 & @Ahmed-Abdelhameed

With all respect for the work already put in.

Also like this PR and especially these changes should be done through dedicated PRs and not mixed in with other work, so we can keep track of what has happened and be able to do a roll-back if applicable.

Earlier announced tests to see if we can fully export the built-in themes (including images) still do not have been reported on e.g. there is no status on that and no knowledge to take forward in the decision making process. All changes need to be compatible throughout the components affected avoiding patchwork.

I stay at the point to first complete the discussion, possible testing, and planning before coding starts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I'm not sure if this is the correct approach.

The earlier understanding or idea was to export the built-in themes to the KCusotmpalettebase format which already can serialize to and from XML.

Probably this serialization in KCPB needs extending and is still up for discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smurf-IV & @giduac

The KryptonThemeLoader will export the built-in themes to *.kthemes in its current state. But I can't find a way to bundle Krypton.Base.Themes.dll into the NuGet package. If either of you have the time, please could you investigate a way of doing this, while I clean the PR's up?

Note: I'll set this as a draft

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I'm not sure if this is the correct approach.

The earlier understanding or idea was to export the built-in themes to the KCusotmpalettebase format which already can serialize to and from XML.

Probably this serialization in KCPB needs extending and is still up for discussion.

So, the new *.ktheme files are essentially the existing XML files, but with extra metadata to accommodate theme name, images & fonts

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I'm saying earlier that the discussion, etc. is not complete and this is floating in an undetermined direction.

@PWagner1 PWagner1 marked this pull request as draft March 22, 2025 08:20
@Smurf-IV
Copy link
Member

Conflicts
And, Please redo this PR with just the changes for the KB shortcuts

@PWagner1 PWagner1 closed this Apr 14, 2025
@PWagner1 PWagner1 deleted the 2134-feature-request-implement-a-way-of-getting-a-way-to-show-keyboard-shortcuts branch April 14, 2025 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants